[MINOR] Migrate RankValue to the package of the common class#265
[MINOR] Migrate RankValue to the package of the common class#265roryqi merged 3 commits intoapache:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #265 +/- ##
============================================
+ Coverage 59.70% 61.10% +1.40%
- Complexity 1377 1516 +139
============================================
Files 166 186 +20
Lines 8916 9408 +492
Branches 853 918 +65
============================================
+ Hits 5323 5749 +426
- Misses 3318 3355 +37
- Partials 275 304 +29
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Please cc, just a minor change. @jerqi |
|
Could you give me a concrete example? Why do we need this pr? |
|
Because this static class is currently in |
|
Why do we migrate RankValue to the package of the common class if shuffle server and client won't use RankValue. |
|
I see what you mean, but I think it is necessary to set it as a separate class. Put it under the coordinator package ? |
Coordinator package is better. |
|
@smallzhongfeng Will you continue this pr? |
Done, sorry to take so long. |
roryqi
left a comment
There was a problem hiding this comment.
LGTM, thanks @smallzhongfeng
What changes were proposed in this pull request?
More standardized, and the class can be extended in the future.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
No need.